fix: default to fetch type in keyed mutator #2753

linkvt · 2023-08-21T15:32:59Z

Hi,

as a few other also noticed the typings used in the bound mutator do not default anymore to the data type used in useSWR<ThisOne> and needs to be specified explicitly everytime right now.

Looking at the other types surrounding the KeyedMutator type it seems reasonable to default to the data type specified at useSWR<...>, e.g. as done here:

swr/_internal/src/types.ts

Lines 372 to 381 in e510955

    
           export interface ScopedMutator { 
        
             /** 
        
              * @typeParam Data - The type of the data related to the key 
        
              * @typeParam MutationData - The type of the data returned by the mutator 
        
              */ 
        
             <Data = any, MutationData = Data>( 
        
               matcher: (key?: Arguments) => boolean, 
        
               data?: MutationData | Promise<MutationData> | MutatorCallback<MutationData>, 
        
               opts?: boolean | MutatorOptions<Data, MutationData> 
        
             ): Promise<Array<MutationData | undefined>>

Let me know if and where a test could be added if you want to test the typings.

Thanks, Vincent

orca-security-us

Orca Security Scan Summary

Status	Check	Issues by priority
Passed	Secrets	0 0 0 0

codesandbox-ci · 2023-08-21T15:33:32Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4b1bd9a:

Sandbox	Source
SWR-Basic	Configuration
SWR-States	Configuration
SWR-Infinite	Configuration
SWR-SSR	Configuration

vercel/swr#2753 これが原因で type-check が落ちてた治ったら上げる

linkvt · 2023-09-05T15:08:01Z

Hello @shuding or @huozhi , would you mind looking at this tiny PR fixing the types of a keyed mutator? I just rebased the code, let me know if something is missing here.
Thanks!

huozhi · 2023-09-05T18:56:58Z

Hi, could you also add a types test for this case so we can avoid the regression later? Thanks

promer94

Thank you!

huozhi · 2023-09-10T11:36:02Z

test/type/mutate.ts

  mutate(async () => 1)

-  mutate(async () => 1, { populateCache: false })
+  // FIXME: this should work.
+  // mutate(async () => 1, { populateCache: false })


@promer94 thanks for adding the test, maybe we should create an issue to follow up on this? it feels like a regression for mutate() API types

The original pr #2708 did not fix this correctly so there is no regression here.

I also think we should not support this because the type implementation would be really complicated and not useful in practice. You should always make sure you data has same structure as cache when using the bounded mutate fn.

I just keep it here as a reference.

right the type is number, I updated them in #2781

AmirHosseinKarimi · 2023-09-24T21:42:04Z

Hey guys. Thanks for improving the SWR.
When we can have these changes on the new version?
Is there any routine release plan?

huozhi · 2023-09-25T09:35:06Z

@AmirHosseinKarimi A few type improvements including this are landed in 2.2.3

ckohen · 2023-10-16T13:22:54Z

_internal/src/types.ts

-    | Promise<MutationData | undefined>
-    | MutatorCallback<MutationData>,
+export type KeyedMutator<Data> = <MutationData = Data>(
+  data?: Data | Promise<Data | undefined> | MutatorCallback<Data>,


@promer94 Why was this change reverted? It makes the generic completely useless and returning different MutationData to be passed to populateCache impossible with proper types.

This probably needs to be overload based to function properly, otherwise MutationData will just be inferred improperly from the data arg, although it is mostly possible to do without overloads with some rather extensive types:

type KeyedMutator<Data> = <MutationData = Data>(data?: MutationData | Promise<MutationData | undefined> | MutatorCallback<MutationData>, opts?: boolean | Omit<MutatorOptions<Data, MutationData>, 'populateCache'> & (MutationData extends Data ? Pick<MutatorOptions<Data, Data>, 'populateCache'> : { populateCache: Exclude<MutatorOptions<Data, MutationData>['populateCache'], boolean> })) => Promise<Data | MutationData | undefined>;

I have tested the above and it works as expected, except in the case where opts is not passed or is a boolean. This is why I suggested the overload approach, since if options is not passed as an object, typescript doesn't enforce it being passed.

simenbrekken-visma · 2023-11-17T07:47:46Z

Not sure if I understood the above changes completely, but after upgrading from 2.0.3 to 2.2.3 we had to update all our populateCache methods:

Before (2.0.3)

const { data = [], mutate } = useSWR<ListItem[]>('/api/list', fetcher);

const addListItem = (name: string) => {
    mutate(
        async () => {
            const item: ListItem = await createListItem(name);

            return item;
        },
        {
            populateCache: (result, currentData = []) => [
                ...currentData,
                result,
            ],
        },
    );
};

After (2.2.3)

const { data = [], mutate } = useSWR<ListItem[]>('/api/list', fetcher);

const addListItem = (name: string) => {
    mutate(
        async () => {
            const item: ListItem = await createListItem(name);

            return [item];
        },
        {
            populateCache: (result, currentData = []) => [
                ...currentData,
                ...result,
            ],
        },
    );
};

We may have missed a generic that can be passed to work around this, but having to modify our mutators to return the same type as given to useSWR feels a bit clunky.

linkvt requested review from shuding and huozhi as code owners August 21, 2023 15:33

orca-security-us bot reviewed Aug 21, 2023

View reviewed changes

SSlime-s added a commit to SSlime-s/qtheme-v2 that referenced this pull request Sep 2, 2023

🆙 SWR のバージョンを落とす

4eacf32

vercel/swr#2753 これが原因で type-check が落ちてた治ったら上げる

linkvt force-pushed the reuse-data-type-in-keyed-mutator branch from 2fb2d29 to f6761db Compare September 5, 2023 15:06

promer94 force-pushed the reuse-data-type-in-keyed-mutator branch from 4d2b12e to 67dab71 Compare September 10, 2023 09:13

promer94 approved these changes Sep 10, 2023

View reviewed changes

linkvt and others added 3 commits September 10, 2023 17:50

fix: default to data type in keyed mutator

e81cbe7

fix: keyedmutator type

d9e412d

test: should use global mutate here

4b1bd9a

promer94 force-pushed the reuse-data-type-in-keyed-mutator branch from 67dab71 to 4b1bd9a Compare September 10, 2023 09:50

promer94 merged commit efe07d7 into vercel:main Sep 10, 2023
4 checks passed

huozhi reviewed Sep 10, 2023

View reviewed changes

linkvt deleted the reuse-data-type-in-keyed-mutator branch September 10, 2023 14:17

ckohen reviewed Oct 16, 2023

View reviewed changes

apiiro-snyk mentioned this pull request Jul 4, 2024

[Snyk] Upgrade swr from 2.1.0 to 2.2.5 apiiro-snyk/ray#5

Open

GCODIN mentioned this pull request Jul 4, 2024

[Snyk] Upgrade swr from 2.2.2 to 2.2.5 GCODIN/docs#10

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: default to fetch type in keyed mutator #2753

fix: default to fetch type in keyed mutator #2753

linkvt commented Aug 21, 2023

orca-security-us bot left a comment

codesandbox-ci bot commented Aug 21, 2023 •

edited

Loading

linkvt commented Sep 5, 2023

huozhi commented Sep 5, 2023

promer94 left a comment •

edited

Loading

huozhi Sep 10, 2023

promer94 Sep 10, 2023

huozhi Sep 12, 2023

AmirHosseinKarimi commented Sep 24, 2023 •

edited

Loading

huozhi commented Sep 25, 2023

ckohen Oct 16, 2023

simenbrekken-visma commented Nov 17, 2023 •

edited

Loading

	export interface ScopedMutator {
	/**
	* @typeParam Data - The type of the data related to the key
	* @typeParam MutationData - The type of the data returned by the mutator
	*/
	<Data = any, MutationData = Data>(
	matcher: (key?: Arguments) => boolean,
	data?: MutationData \| Promise<MutationData> \| MutatorCallback<MutationData>,
	opts?: boolean \| MutatorOptions<Data, MutationData>
	): Promise<Array<MutationData \| undefined>>

fix: default to fetch type in keyed mutator #2753

fix: default to fetch type in keyed mutator #2753

Conversation

linkvt commented Aug 21, 2023

orca-security-us bot left a comment

Choose a reason for hiding this comment

Orca Security Scan Summary

codesandbox-ci bot commented Aug 21, 2023 • edited Loading

linkvt commented Sep 5, 2023

huozhi commented Sep 5, 2023

promer94 left a comment • edited Loading

Choose a reason for hiding this comment

huozhi Sep 10, 2023

Choose a reason for hiding this comment

promer94 Sep 10, 2023

Choose a reason for hiding this comment

huozhi Sep 12, 2023

Choose a reason for hiding this comment

AmirHosseinKarimi commented Sep 24, 2023 • edited Loading

huozhi commented Sep 25, 2023

ckohen Oct 16, 2023

Choose a reason for hiding this comment

simenbrekken-visma commented Nov 17, 2023 • edited Loading

codesandbox-ci bot commented Aug 21, 2023 •

edited

Loading

promer94 left a comment •

edited

Loading

AmirHosseinKarimi commented Sep 24, 2023 •

edited

Loading

simenbrekken-visma commented Nov 17, 2023 •

edited

Loading